refactor: unify LinkNavigator truth-link handling - #2780

src/algorithms/onnx/CalorimeterParticleIDPostML.cc:36

• Throwing std::runtime_error("") loses the useful context you already logged. Include a non-empty exception message (e.g., same text as the log line) to make failures actionable in contexts where logs aren’t captured.

  if (prediction_tensors->size() != 1) {
    error("Expected to find a single tensor, found {}", prediction_tensors->size());
    throw std::runtime_error("");
  }

src/algorithms/onnx/CalorimeterParticleIDPreML.cc:63

• The !found_assoc branch warns and continues, but the tensor shapes are still set to clusters->size(), so skipping any cluster guarantees a later shape/element-count mismatch (and an exception). Either resize tensors to the number of filled entries or fail fast when a cluster has no linked MCParticle.

    if (!found_assoc) {
      warning("Can't find association for cluster. Skipping...");
      continue;
    }

src/algorithms/onnx/CalorimeterParticleIDPreML.h:21

• inputClusterLinks is modeled as std::optional<...> in the algorithm signature, but the implementation treats it as required (throws when null/empty) and the factory always wires it as a mandatory PodioInput. Make the requirement explicit in the algorithm type to avoid a misleading optional contract.

using CalorimeterParticleIDPreMLAlgorithm = algorithms::Algorithm<
    algorithms::Input<edm4eic::ClusterCollection,
                      std::optional<edm4eic::MCRecoClusterParticleLinkCollection>>,
    algorithms::Output<edm4eic::TensorCollection, std::optional<edm4eic::TensorCollection>>>;

src/algorithms/fardetectors/FarDetectorTransportationPreML.cc:85

• When trackLinks is present but empty (or a given track has no linked MCParticle), target_tensor still advertises a shape of inputTracks->size() x 3 but no (or fewer) elements are appended. This breaks the stated goal of keeping indexing aligned and can produce inconsistent tensors downstream. Ensure you append 3 values per track whenever the target tensor is created (e.g., NaNs when no link exists).

    if (link_nav) {
      // Use the first linked MC particle, matching previous first-association behavior.
      const auto linked_particles = link_nav->getLinked(track);
      if (!linked_particles.empty()) {
        auto MCElectronMomentum = linked_particles.front().o.getMomentum() / m_beamE;
        target_tensor.addToFloatData(MCElectronMomentum.x);
        target_tensor.addToFloatData(MCElectronMomentum.y);
        target_tensor.addToFloatData(MCElectronMomentum.z);
      }
    }

src/algorithms/onnx/CalorimeterParticleIDPreML.cc:45

• target_tensors is an optional output (see algorithm signature) but is dereferenced unconditionally. If the optional target output isn’t wired, this will crash. Also, cluster_links is optional in the input contract but treated as required here; at minimum this should fail fast with a clear error instead of null-dereferencing later.

  if (cluster_links == nullptr || cluster_links->empty()) {
    error("Cluster links are required for CalorimeterParticleIDPreML");
    throw std::runtime_error("Missing inputClusterLinks");
  }

  edm4eic::MutableTensor feature_tensor = feature_tensors->create();
  feature_tensor.addToShape(clusters->size());
  feature_tensor.addToShape(11);    // p, E/p, azimuthal, polar, 7 shape parameters
  feature_tensor.setElementType(1); // 1 - float

  edm4eic::MutableTensor target_tensor;
  target_tensor = target_tensors->create();
  target_tensor.addToShape(clusters->size());
  target_tensor.addToShape(2);     // is electron, is hadron
  target_tensor.setElementType(7); // 7 - int64

src/algorithms/onnx/CalorimeterParticleIDPreML.cc:63

• This path skips clusters without adding any feature/target entries, but the tensor shape was pre-sized to clusters->size(). That guarantees a shape/data mismatch and an exception later, but with a much less actionable error message.

    if (!found_assoc) {
      warning("Can't find association for cluster. Skipping...");
      continue;
    }

src/algorithms/fardetectors/FarDetectorTransportationPreML.cc:60

• target_tensor is created whenever trackLinks != nullptr, even if the collection is empty (or the optional output isn’t wired). That can leave target_tensor with a non-zero shape but no data appended in the loop, producing inconsistent output.

  edm4eic::MutableTensor target_tensor;
  if (trackLinks != nullptr) {
    target_tensor = target_tensors->create();
    target_tensor.addToShape(inputTracks->size());
    target_tensor.addToShape(3);     // px,py,pz
    target_tensor.setElementType(1); // 1 - float
  }

  std::optional<podio::LinkNavigator<edm4eic::MCRecoTrackParticleLinkCollection>> link_nav;
  if (trackLinks != nullptr && !trackLinks->empty()) {
    link_nav.emplace(*trackLinks);
  }

src/algorithms/fardetectors/FarDetectorTransportationPreML.cc:85

• When link_nav is enabled but a particular track has no linked MC particle, no target values are appended for that track. This breaks the implicit 1:1 alignment between feature rows and target rows.

    if (link_nav) {
      // Use the first linked MC particle, matching previous first-association behavior.
      const auto linked_particles = link_nav->getLinked(track);
      if (!linked_particles.empty()) {
        auto MCElectronMomentum = linked_particles.front().o.getMomentum() / m_beamE;
        target_tensor.addToFloatData(MCElectronMomentum.x);
        target_tensor.addToFloatData(MCElectronMomentum.y);
        target_tensor.addToFloatData(MCElectronMomentum.z);
      }
    }

src/algorithms/fardetectors/FarDetectorTransportationPreML.cc:55

• target_tensor is created whenever trackLinks is non-null, even if the collection is empty. In that case link_nav is never constructed and no elements are appended, leaving a tensor with shape based on inputTracks->size() but with no data. Create the target tensor only when the link collection is non-empty (or when the navigator is enabled).

  edm4eic::MutableTensor target_tensor;
  if (trackLinks != nullptr) {
    target_tensor = target_tensors->create();
    target_tensor.addToShape(inputTracks->size());
    target_tensor.addToShape(3);     // px,py,pz
    target_tensor.setElementType(1); // 1 - float
  }

src/algorithms/onnx/CalorimeterParticleIDPreML.cc:63

• The missing-association branch logs a warning and continues, but the output tensor shapes are still set to clusters->size(), and a consistency check at the end will always throw if any cluster was skipped. Either handle missing links by emitting placeholder feature/target entries to preserve the fixed shape, or fail fast with a clear error instead of warning+continuing.

    if (!found_assoc) {
      warning("Can't find association for cluster. Skipping...");
      continue;
    }

src/algorithms/onnx/CalorimeterParticleIDPreML.h:21

• The algorithm signature declares inputClusterLinks as std::optional<...>, but the implementation treats links as required (throws when missing/empty). Consider making the input non-optional to reflect the contract and avoid confusing wiring/configuration.

using CalorimeterParticleIDPreMLAlgorithm = algorithms::Algorithm<
    algorithms::Input<edm4eic::ClusterCollection,
                      std::optional<edm4eic::MCRecoClusterParticleLinkCollection>>,
    algorithms::Output<edm4eic::TensorCollection, std::optional<edm4eic::TensorCollection>>>;

src/algorithms/onnx/CalorimeterParticleIDPreML.h:21

• CalorimeterParticleIDPreML declares the cluster-link input as optional in the algorithm interface, but the implementation now hard-requires it (throws when null/empty). This makes the API contract misleading for callers and wiring; it should be modeled as a required input type instead of std::optional<...>.

using CalorimeterParticleIDPreMLAlgorithm = algorithms::Algorithm<
    algorithms::Input<edm4eic::ClusterCollection,
                      std::optional<edm4eic::MCRecoClusterParticleLinkCollection>>,
    algorithms::Output<edm4eic::TensorCollection, std::optional<edm4eic::TensorCollection>>>;

src/algorithms/onnx/CalorimeterParticleIDPreML.cc:64

• This branch says it is "Skipping" clusters with no truth link, but the tensors were already sized to clusters->size(). Skipping here guarantees a shape/data mismatch (and a later throw) when any cluster lacks a link. Either pre-filter valid clusters before sizing, or fail fast here with a clear error so the tensor shape remains consistent.

    if (!found_assoc) {
      warning("Can't find association for cluster. Skipping...");
      continue;
    }

src/algorithms/fardetectors/FarDetectorTransportationPreML.cc:77

• target_tensor is shaped to inputTracks->size(), but when a given track has no linked MC particle this loop adds no target entries for that track. That silently produces a tensor whose declared shape doesn't match its data length (and de-aligns feature/target rows). Add a fallback (e.g., 0/0/0 or NaNs) so exactly 3 floats are appended per input track, and avoid relying on .front() if the underlying linked-range type isn't guaranteed to provide it.

    if (link_nav.enabled()) {
      // Use the first linked MC particle, matching previous first-association behavior.
      const auto linked_particles = link_nav.linked(track);
      if (!linked_particles.empty()) {
        auto MCElectronMomentum = linked_particles.front().o.getMomentum() / m_beamE;

src/algorithms/fardetectors/FarDetectorTransportationPreML.cc:77

• When link_nav.enabled() is true, target_tensor is shaped for inputTracks->size() tracks, but entries are only appended when a given track has at least one linked MC particle. If linked_particles is empty for any track, the target tensor will have fewer than N*3 elements and become inconsistent/misaligned downstream. To keep tensor shape/data aligned, append a placeholder (e.g., zeros) for tracks without a linked particle, or throw if missing links are not allowed.

    if (link_nav.enabled()) {
      // Use the first linked MC particle, matching previous first-association behavior.
      const auto linked_particles = link_nav.linked(track);
      if (!linked_particles.empty()) {
        auto MCElectronMomentum = linked_particles.front().o.getMomentum() / m_beamE;

src/algorithms/fardetectors/FarDetectorTransportationPreML.cc:79

• target_tensor is shaped as inputTracks->size(), but for tracks with no linked MC particle this code adds no target values. That can silently create a tensor whose shape/data length disagree and misalign track indices downstream. If targets are enabled, write 3 values per track (e.g., zeros) even when no link exists, or throw to make the invariant explicit.

      const auto linked_particles = link_nav.linked(track);
      if (!linked_particles.empty()) {
        auto MCElectronMomentum = linked_particles.front().o.getMomentum() / m_beamE;
        target_tensor.addToFloatData(MCElectronMomentum.x);
        target_tensor.addToFloatData(MCElectronMomentum.y);

src/global/pid_lut/pid_lut.cc:11

• pid_lut.cc includes podio/detail/Link.h and <deque>, but neither is used in this file. podio/detail/* headers are typically internal and can break builds across podio versions; removing these unused includes reduces fragility.
  src/algorithms/onnx/CalorimeterParticleIDPreML.cc:63
• When fill_targets is true but a cluster has no linked MC particle, the code logs "Skipping..." and continues. This guarantees feature_tensor.floatData_size() will not match the predeclared shape (clusters->size()), so the function will later throw an "Inconsistent output tensor shape" error anyway. Consider failing fast with a clear error here (or alternatively redesigning the tensor shape to only include kept clusters).

      if (!found_assoc) {
        warning("Can't find association for cluster. Skipping...");
        continue;
      }

src/algorithms/fardetectors/FarDetectorTransportationPreML.cc:77

• This block can produce an inconsistent target tensor: when a track has no linked MC particle, no (px,py,pz) entries are appended even though the tensor shape is set to inputTracks->size(). It also uses .front() on the linked range, which is brittle if the return type is not a container. Consider iterating the linked range once, taking the first element, and always appending 3 values (e.g., zeros) to keep indexing aligned.

    if (link_nav.enabled()) {
      // Use the first linked MC particle, matching previous first-association behavior.
      const auto linked_particles = link_nav.linked(track);
      if (!linked_particles.empty()) {
        auto MCElectronMomentum = linked_particles.front().o.getMomentum() / m_beamE;

src/algorithms/onnx/CalorimeterParticleIDPreML.cc:33

• feature_tensor always uses momentum (and E/p), but when the link collection is missing/empty the current logic leaves momentum as NaN and silently writes NaNs to the feature tensor. Also, target_tensors is optional but is dereferenced without a null check. Consider (a) failing fast when links are missing/empty since momentum cannot be computed yet (per the FIXME), and (b) only creating/filling the target tensor when target_tensors is present.

  const truth::EventLinkNavigator<edm4eic::MCRecoClusterParticleLinkCollection> link_nav(
      cluster_links);
  const bool fill_targets = link_nav.enabled();

  edm4eic::MutableTensor feature_tensor = feature_tensors->create();

src/algorithms/onnx/CalorimeterParticleIDPreML.cc:49

• Momentum is currently computed only when fill_targets is true, but the feature tensor always consumes momentum (and E/p). If outputTargetTensor is disabled (so fill_targets becomes false after the suggested guard), this will again emit NaNs into the feature tensor. Momentum should be computed whenever links are available (independent of whether targets are written).

    if (fill_targets) {

src/global/pid_lut/pid_lut.cc:11

• <podio/detail/Link.h> is an internal PODIO header and appears unused here, and <deque> is also unused. Depending on PODIO internals makes builds more fragile across PODIO upgrades; please remove these unused/internal includes if they are not required.